fix: warn when an asset is skipped at the package boundary - #607
Open
puneet-ekline wants to merge 1 commit into
Open
fix: warn when an asset is skipped at the package boundary#607puneet-ekline wants to merge 1 commit into
puneet-ekline wants to merge 1 commit into
Conversation
The package-boundary guard added in vercel#568 stops a package inside node_modules from emitting assets outside node_modules. That is deliberate, but the skip is currently only visible under `job.log`, so in a normal build the file is simply absent from the output and the first symptom is a runtime failure in the deployed application. Surface it in `warnings` as well, so consumers can see and act on it at build time. Behaviour is unchanged: the asset is still not emitted. This cost us a multi-hour production outage (see vercel#606). next-i18next resolves its user config from the current working directory: const configPath = path.resolve('./next-i18next.config.js'); if (!userConfig && fs.existsSync(configPath)) { userConfig = await import(configPath); } Babel compiles that dynamic import to a Promise wrapper around `require`, which is analysed as an asset rather than a dependency, so the path goes through emitAssetPath and is skipped. The config never reached the serverless bundle, `existsSync` returned false, and next-i18next threw before any page rendered -- with nothing in the build output to indicate why. The new fixture covers the case the existing *-outside-base fixtures do not: their `/../../secret.txt` normalises to a path that does not exist, so it returns at the stat check before reaching this guard. Tests: 1375 passed (1373 before, plus the two variants of the new fixture).
puneet-ekline
requested review from
a team,
icyJoseph,
ijjk and
styfle
as code owners
August 11, 2026 23:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Used AI to investigate and propose this fix.
Closes #606 (partially — see "What this does not change" below).
Problem
The package-boundary guard added in #568 stops a package inside
node_modulesfrom emitting assets outsidenode_modules. The skip is currently only visible underjob.log, so in a normal build the file is simply absent from the output and the first symptom is a runtime failure in the deployed application.This cost us a multi-hour production outage.
next-i18nextresolves its user config from the current working directory:Babel compiles that dynamic
import()into a Promise wrapper aroundrequire, which is analysed as an asset rather than a dependency, so the path flows throughemitAssetPathand hits the guard. After Next.js 16.3.0 picked up a post-1.3.2 nft, the config stopped reaching the serverless bundle,existsSyncreturned false, and next-i18next threw before any page rendered — 500ing every server-rendered response, including the on-demand 404.next buildexited 0 throughout, and there was nothing in the output to indicate why.Looking up a config file from
process.cwd()is a widespread convention, so this likely reaches beyondnext-i18next.What this changes
The skip is added to
warningsin addition to the existingjob.logoutput, so consumers can surface it at build time.What this does not change
Behaviour is identical — the asset is still not emitted, and no existing expectation changes. I have deliberately not touched the boundary rule itself: the fixtures added in #568 assert it, so whether in-base assets should be skipped is a maintainer decision. That question is #606; this PR only makes the outcome observable either way.
New test fixture
test/unit/pkg-cwd-asset-outside-pkg-basecovers a case the existing*-outside-basefixtures do not. Those use/../../secret.txt, which normalises to/secret.txt; that path does not exist, so tracing returns at the stat check before reaching this guard — they pass without exercising it. Verified by instrumenting the branch: neither fixture reaches it.The new fixture uses the Babel-transpiled dynamic import of a cwd-resolved path, which does reach the guard, and asserts:
fileList(behaviour preserved)The assertion is scoped to the uncached pass, since the second run replays cached analysis and does not re-emit warnings.
Verification
jest test/unit.test.js test/ecmascript.test.js: 1375 passed (1373 before, plus the two variants of the new fixture)prettier --checkclean on all touched filessrc/analyze.tsalone makes it failnext-i18next'sserverSideTranslationsentry now yields a warning namingnext-i18next.config.js, where previously the file was dropped with no signalecmascript.test.jsassertswarnings.size === 0, and it still passes, so no existing fixture triggers this path.